Skip to content
This repository was archived by the owner on Nov 21, 2019. It is now read-only.

Conversation

@thewilkybarkid
Copy link
Contributor

Requires use statements (so no FCQN and no global fallbacks).

@thewilkybarkid thewilkybarkid added the feature New feature or request label Sep 25, 2018

namespace Vendor;

new Foo();
Copy link
Contributor Author

@thewilkybarkid thewilkybarkid Sep 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, as the use statement is being removed. One of the new sniffs must be misbehaving. Edit: It's UseFromSameNamespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it, UseFromSameNamespace is actually correct because the use statement is in the global namespace, where it's not needed. Would still rather UseBeforeNamespace is triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use a namespaced class to get round this for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just checked what happens and the code's going to be broken either way.)

@thewilkybarkid thewilkybarkid changed the title [WIP] Use statements must be used Use statements must be used Sep 25, 2018
@thewilkybarkid thewilkybarkid requested a review from a team October 1, 2018 09:04
Copy link

@stephenwf stephenwf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats a really useful auto-fixer 👍

@@ -0,0 +1,25 @@
---DESCRIPTION---
Use statements must not be from the same namespace

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good practice

@@ -0,0 +1,32 @@
---DESCRIPTION---
Use statements must be used

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for classes, this is expected


namespace Vendor;

use function date;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see this becoming annoying for many PHP functions e.g. all strlen and similar which gets used everywhere. Even Java auto-import java.lang to avoid too many imports. PHP provides the fallback too for functions (but not for classes IIRC).

If anything I would forbid defining a strlen that is not \strlen, which tries to avoid the ambiguity.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some functions get a very-minor performance boost by not checking the fallback. But more importantly prevents https://www.giorgiosironi.com/2010/09/monkey-patching-in-php.html (as used by Symfony's ClockMock).

Might be annoying when not using an IDE though...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thewilkybarkid thewilkybarkid added this to the 0.2.0 milestone Oct 1, 2018
@thewilkybarkid thewilkybarkid merged commit 80dd646 into libero:master Oct 2, 2018
@thewilkybarkid thewilkybarkid deleted the require-use branch October 2, 2018 13:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants